able to config bert scorer for gpu/cpu, as well as cache it#281
Conversation
dmjoy
left a comment
dmjoy
left a comment
There was a problem hiding this comment.
I think I'd much prefer the bert scorer / similarly piece to be a proper class / object if possible. Or is there any reason why we can't initialize the BERTScorer instance when the ICL component is initialized (as self.bert_scorer or something) with sensible defaults (so they don't need to be specified in most cases) i.e. device: "auto", and then just pass that instance to the bert_similarity_selection calls? Though I'll admit I'm not intimately familiar with the ICL code. Tagging @eveenhuis as she may have a more informed opinion here.
| bert_scorer_device: auto | ||
| cache_bert_scorer: true |
There was a problem hiding this comment.
I would prefer an implementation where retroactively editing configs isn't needed
| Returns: | ||
| BERTScorer instance on the requested device | ||
| """ | ||
| global _bert_scorer, _bert_scorer_device |
There was a problem hiding this comment.
Thinking we can arrive at a solution that doesn't require globals. Will include details on my suggested approach in the main review text
| return _bert_scorer | ||
|
|
||
|
|
||
| def bert_similarity_selection(candidates, texts_to_compare, reference_text, n_examples, score_adjustments=None, least_similar_examples=False, bert_scorer_device="auto", cache_bert_scorer=True): |
There was a problem hiding this comment.
Similarly I'm not a fan of how we have to now pass around bert_scorer_device and cache_bert_scorer parameters in several places
| incontext_settings, | ||
| target_kdmas, | ||
| state_hydration_domain=None, | ||
| scenario_description_template=None, |
There was a problem hiding this comment.
I.e. have a new keyword argument here for scorer=None; then in the __init__ body initialize a BERTScorer (or wrapper object) and set to something like self.scorer then pass that to the function for determining matches.
2 participants
No description provided.